I have seen the closed PR #3411 which extracts the single apply diff into its own function, i think that could be a good addition for reducing complexity too.
Thoughts?

@Ruakij is attempting to deploy a commit to the Roo Code Team on Vercel.

A member of the Team first needs to authorize it.

Hey @Ruakij, thanks for working on this! I've been looking through the changes and I appreciate the modular approach you've taken.

I'm a bit concerned though - this feels like quite a complex solution for something that mainly affects DeepSeek models (DeepSeek R1 to be more specific). The engine adds processing to every search/replace operation, and I worry it might interfere with cases where someone actually wants to duplicate similar code patterns.

Since the root issue seems to be that certain LLMs include existing closing braces in their replacements, what do you think about trying something simpler first? Maybe we could improve the prompts for those specific models, or add a quick check that warns when a replacement would create syntax errors?

I'm curious if you considered any lighter-weight approaches before going with the engine solution? Would love to hear your thoughts on this!

I think there is always a danger when trying to fix issues some LLMs might have or how their behaviour is. I have had this replacement issue with Sonnet models multiple times now too.
When writing code they are trained to rather complete the block properly rather than following the overall high-level structure. This is therefore probably an issue for any structured output they generate.

My approach is pretty primitive, but should cover most scenarios, as long as replacement lines arent similar on purpose. Not 100% sure how to detect that reliably.

For structured languages with grammar-rules we can parse, at least we can detect if either the LLM or our fix made a mistake, so if we really want to keep the fix broad but reduce errors, the ideal approach would be to parse the result first and check if syntax errors were introduced at the edges.
Though this would require introducing a parsing step ourselves, something the IDE will most likely already run. Quite complex stuff to handle.
If we truly want auto-fixing, this can be done in limited capacity with heuristics, but for catching other errors it could be better to use an apply-diff-model like some others.

So for now, maybe keeping the SuperfluousDuplicatedLineEngine is okay, but adding some checks so we dont accidentally touch stuff we didnt want "fixed". e.g. We only consider lines "similar" if they have at least 3 characters (including spaces), but we wont apply any fixes when we find more than 2 similar lines. (or % of output)
Currently we would cut off any similar lines, no matter what they are (ignoring spaces) or how many.

@Ruakij
What we want to avoid is to add checks and guardrails for every problem and quirk a model might have, it's better to first explore minimal changes, changes to the way the tool is described, basically attack the root problem rather than trying to attack the symptoms.

Checking every apply_diff just to see if a single { is left behind seems like a bit of a waste, specially since we are always improving our system prompt and models get better at following instructions, making our guardrails obsolete.

I'll close this PR for now but that doesn't mean I am not open to discussing this further, please let me know what you think!